Modified class for additional functionality#4
Conversation
|
Hi - I've been traveling this week, so I haven't had a chance to look at this. I'll try to take a look next week |
JSAnimation/html_writer.py
Outdated
There was a problem hiding this comment.
All of tabs should be four spaces -- it can cause problems to mix indentation levels like this.
|
Hi, Can you try working from the current master, making sure to use four tabs, so that I can see what your changes are? Thanks Oh - and sorry for taking so long to look at this! I really do appreciate the work you're doing here. |
|
Hi Jake Thanks for all your input! I'll fork the latest version and try to add the changes there. I'll send you another pull request when it's ready. Cheers, On Aug 5, 2013, at 10:46 AM, Jake Vanderplas notifications@github.com wrote:
|
|
Hi Jake, I made the changes directly into your updated version. The PR should be easier to understand now. Let me know what you think. |
There was a problem hiding this comment.
I'd prefer not to hard-code the frame width here. If we create a figure with a given size in matplotlib, this size should be reflected in the resulting animation.
|
Lots of nitpicks -- sorry about that! In general it looks pretty good. I'd be happier if functions and variables were named in a way that didn't mislead the reader -- more detailed comments above. |
|
Hi Jake, Thanks for your comments! I did apply some of your recommendations, and that's the version that Cheers,
|
2 participants
Hi Jake,
It seems I was using an old version of your JSAnimation, so I didn't have some of your final adjustments. I can try to add the changes to your newer version and do a new pull request (as i should have done from the beginning), though you might be able to merge them without confilcts.
The changes I made were: frame number box addition in animation windows, new function to generate general filenames, frame directory control, animation speed control, frame width control and option for additional html code in the preamble.
I don't know if all these changes are relevant, but they were very useful in order to implement it into Clawpack (also I like the new framebox). I made all the changes carefully to maintain ipython notebook compatibility.
Let me know what you think,
Mauricio